[wasm][coreclr] 2nd part of call helpers generator, reverse calls #122007

radekdoulik · 2025-11-26T21:47:37Z

The 2nd part generates reverse thunks for methods decorated with UnmanagedCallersOnlyAttribute. It also generates entrypoints when specified by the attribute.

These methods can be called either through the entrypoint or as function pointers (from IL or native code).

Sample code that will use the generated callhelpers:

using System.Runtime.InteropServices;

unsafe {
    var fn = (delegate* unmanaged<void>)&UMCOMethod;

    fn();
}

[UnmanagedCallersOnly]
static void UMCOMethod()
{
    Console.WriteLine("Hello from UMCOMethod");
}

The calls using function pointers use hashing tables with keys and fallback keys. The primary key is used when the assembly containing the UMCO method hasn't changed, and the key is based on assembly FQN and method token. The fallback key is used when the assembly has changed (for example during trimming), and is based on assembly name, method name, namespace, type, and number of parameters.

Add fallback key

dotnet-policy-service · 2025-11-26T21:48:27Z

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR implements the second part of the call helpers generator for WASM and CoreCLR, focusing on reverse thunks for methods decorated with UnmanagedCallersOnlyAttribute. The implementation enables native code to call managed methods through generated C++ thunks, using a dual hash-based lookup system with primary keys (based on assembly FQN and method token) and fallback keys (based on method properties) to handle assembly changes during trimming.

Key changes:

Adds new PInvokeTableGenerator and PInvokeCollector classes for both mono and coreclr runtimes
Implements hash-based reverse thunk lookup with fallback mechanism in the CoreCLR VM
Generates C++ reverse thunks with lazy MethodDesc lookup and proper argument marshaling

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/tasks/WasmAppBuilder/mono/PInvokeTableGenerator.cs`	New generator for mono runtime that creates PInvoke tables and native-to-interp entry functions
`src/tasks/WasmAppBuilder/mono/PInvokeCollector.cs`	New collector for mono that scans assemblies for PInvoke and callback methods
`src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs`	New generator for CoreCLR that creates reverse thunks with dual-key hash table entries
`src/tasks/WasmAppBuilder/coreclr/PInvokeCollector.cs`	New collector for CoreCLR that scans assemblies and builds callback metadata
`src/tasks/WasmAppBuilder/coreclr/SignatureMapper.cs`	Adds `TypeToNameType` helper for converting types to name strings in generated code
`src/tasks/WasmAppBuilder/WasmAppBuilder.csproj`	Updates target framework and output path configurations for the generator task
`src/coreclr/vm/wasm/helpers.cpp`	Implements dual-key lookup mechanism with primary and fallback hash tables
`src/coreclr/vm/wasm/callhelpers.hpp`	Adds fallbackKey field to ReverseThunkMapEntry structure
`src/coreclr/vm/wasm/callhelpers-reverse.cpp`	Generated reverse thunks with lazy MethodDesc lookup for UnmanagedCallersOnly methods
`src/coreclr/vm/wasm/callhelpers-interp-to-managed.cpp`	Removes duplicate entry in thunk table

src/tasks/WasmAppBuilder/coreclr/PInvokeCollector.cs

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

src/coreclr/vm/wasm/helpers.cpp

src/coreclr/vm/wasm/callhelpers-reverse.cpp

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

Co-authored-by: Copilot <[email protected]>

src/tasks/WasmAppBuilder/WasmAppBuilder.csproj

Co-authored-by: Adeel Mujahid <[email protected]>

…sm-callhelpers-generator-2nd-part

kg · 2025-12-01T16:46:11Z

src/coreclr/vm/wasm/helpers.cpp

+        // Try primary key, it is based on Assembly fully qualified name and method token
        const ReverseThunkMapValue* thunk;
+        if (table->Lookup(key, &thunk))
+        {
+            return thunk;
+        }
+
+        // Try fallback key, that is based on method properties and assembly name
+        // The fallback is used when the assembly is trimmed and the token and assembly fully qualified name
+        // may change.
+        table = VolatileLoad(&reverseThunkFallbackCache);
+        if (table == nullptr)
+        {
+            table = CreateReverseThunkHashTable(true /* fallback */);
+        }
+
+        key = CreateFallbackKey(pMD);
+


What happens if we get a key collision? Is it mechanically prevented from happening somewhere?

Looking deeper, I'm concerned that we're using a low quality 32-bit hash here, so I would hope that at least if we get a hash collision during thunk lookup, it will fail deterministically with a clear error message so we can go back and fix it. Ideally we would use a more robust 64-bit hash to make the odds of a collision way lower, or do an actual string check. But I don't know if thunk lookup is on a hot path to the point that we can't afford it.

kg · 2025-12-01T16:53:42Z

src/tasks/WasmAppBuilder/coreclr/PInvokeCollector.cs

+            string.Equals(EntryPoint, other.EntryPoint, StringComparison.Ordinal) &&
+            string.Equals(Module, other.Module, StringComparison.Ordinal) &&
+            string.Equals(Method.ToString(), other.Method.ToString(), StringComparison.Ordinal);
+


I think this should override object.Equals too just to be safe

I'd also prefer to see GetHashCode live here instead of in the comparer.

kg · 2025-12-01T16:54:43Z

src/tasks/WasmAppBuilder/coreclr/PInvokeCollector.cs

+        => other != null &&
+            string.Equals(EntryPoint, other.EntryPoint, StringComparison.Ordinal) &&
+            string.Equals(Module, other.Module, StringComparison.Ordinal) &&
+            string.Equals(Method.ToString(), other.Method.ToString(), StringComparison.Ordinal);


Should we be checking WasmLinkage too?

kg · 2025-12-01T17:26:33Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+
+internal sealed class PInvokeTableGenerator
+{
+    private readonly Dictionary<Assembly, bool> _assemblyDisableRuntimeMarshallingAttributeCache = new();


Nit: PInvokeCollector has this cache too, can we unify them?

kg · 2025-12-01T17:44:27Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+        using TempFileName tmpFileName = new();
+        using (var w = new JoinedStringStreamWriter(tmpFileName.Path, false))
+        {
+            // WASM-TODO:


An explanation of why it's commented out or when we can uncomment it would be cool

kg · 2025-12-01T17:45:14Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+                // Special case for __Internal and * modules to indicate static linking without specifying the module
+                modules.Add(pinvoke.Module, pinvoke.Module);
+                Log.LogMessage(MessageImportance.Low, $"Adding module {pinvoke.Module} for static linking");
+            }


Should we have an else here that logs a message that this pinvoke was ignored?

AaronRobinsonMSFT · 2025-12-01T17:19:45Z

src/tasks/WasmAppBuilder/coreclr/PInvokeCollector.cs

+
+#pragma warning disable CA1067
+#pragma warning disable CS0649
+internal sealed class PInvoke : IEquatable<PInvoke>


I think this is precisely what C# records were designed for.

/cc @agocke

Yeah, these are a good candidate for record class. Note that records will check all members when comparing, which might be a problem for Skip and WasmLinkage since right now we don't compare them.

AaronRobinsonMSFT · 2025-12-01T17:23:27Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+            #include <mono/utils/details/mono-error-types.h>
+            #include <mono/metadata/assembly.h>
+            #include <mono/utils/mono-error.h>
+            #include <mono/metadata/object.h>
+            #include <mono/utils/details/mono-logger-types.h>


Are these correct for coreclr?

AaronRobinsonMSFT · 2025-12-01T17:24:22Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+            var imports = pinvokes
+                .Where(l => l.Module == module && !l.Skip)
+                .OrderBy(l => l.EntryPoint, StringComparer.Ordinal)
+                .GroupBy(d => d.EntryPoint, StringComparer.Ordinal)
+                .Select(l => $"{{\"{EscapeLiteral(l.Key)}\", {CEntryPoint(l.First())}}}, // {ListRefs(l)}{w.NewLine}    ")


This could use a comment. Is the order and grouping important?

AaronRobinsonMSFT · 2025-12-01T17:25:36Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+        var moduleImports = new Dictionary<string, List<string>>();
+        foreach (var module in modules.Keys)
+        {
+            static string ListRefs(IGrouping<string, PInvoke> l) =>


Local function should be at the end of the method.

AaronRobinsonMSFT · 2025-12-01T17:27:02Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+            return candidates
+                        .Skip(1)
+                        .Any(c => !TryIsMethodGetParametersUnsupported(c.Method, out _) &&
+                                    c.Method.GetParameters().Length != firstNumArgs);


Please comment on the intent here.

AaronRobinsonMSFT · 2025-12-01T17:31:22Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+
+    // FIXME: System.Reflection.MetadataLoadContext can't decode function pointer types
+    // https://github.com/dotnet/runtime/issues/43791
+    private static bool TryIsMethodGetParametersUnsupported(MethodInfo method, [NotNullWhen(true)] out string? reason)


Please flip this logic to be TryIsMethodGetParametersSupported and then in the call you can negate it. Most try logic is about valid state, unsupported is not "valid".

AaronRobinsonMSFT · 2025-12-01T17:32:55Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+    private static string? EscapeLiteral(string? input)
+    {
+        if (input == null)
+            return null;


Suggested change

private static string? EscapeLiteral(string? input)

{

if (input == null)

return null;

private static string EscapeLiteral(string? input)

{

if (input == null)

return string.Empty;

Is it possible to remove the string? by replacing with string.Empty?

AaronRobinsonMSFT · 2025-12-01T17:45:14Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+    public static bool IsFunctionPointer(Type type)
+    {
+        object? bIsFunctionPointer = type.GetType().GetProperty("IsFunctionPointer")?.GetValue(type);
+        return (bIsFunctionPointer is bool b) && b;
+    }


Why is this needed? I'm assuming we can target .NET 10.

This looks like legacy code that was copied over from the mono based tasks that needed to support old SRM

kg · 2025-12-01T17:46:23Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+            }
+        }
+
+        w.WriteLine(


nit: I'm aware that the call sites for this seem to be commented out but we probably should try to avoid checking in references to mono stuff if we can. On the other hand this isn't a problem since the moment we start using it, it will break in an obvious way

kg · 2025-12-01T17:51:10Z

src/tasks/WasmAppBuilder/coreclr/PInvokeTableGenerator.cs

+        return sb.ToString();
+    }
+
+    // this is eqivalent to `ULONG HashString(LPCWSTR szStr)` in CoreCLR runtime


Nit: Please be more specific in this comment by specifying which file it's from

kg

Thanks for doing the painful work to make this happen

radekdoulik added 3 commits November 18, 2025 18:08

[wasm][coreclr] 2nd part of call helpers generator, reverse calls

c8c3fc0

Do not target NETFramework anymore

a3f20dc

Update keys calculation

89556d8

Add fallback key

radekdoulik added this to the Future milestone Nov 26, 2025

radekdoulik requested review from AaronRobinsonMSFT and jkotas November 26, 2025 21:47

radekdoulik requested a review from maraf as a code owner November 26, 2025 21:47

Copilot AI review requested due to automatic review settings November 26, 2025 21:47

radekdoulik requested a review from akoeplinger as a code owner November 26, 2025 21:47

radekdoulik added the area-CodeGen-Interpreter-coreclr label Nov 26, 2025

dotnet-policy-service bot assigned radekdoulik Nov 26, 2025

Copilot started reviewing on behalf of radekdoulik November 26, 2025 21:48 View session

Copilot finished reviewing on behalf of radekdoulik November 26, 2025 21:50

Copilot AI reviewed Nov 26, 2025

View reviewed changes

Feedback

270a506

Co-authored-by: Copilot <[email protected]>

am11 reviewed Nov 27, 2025

View reviewed changes

src/tasks/WasmAppBuilder/WasmAppBuilder.csproj Outdated Show resolved Hide resolved

am11 reviewed Nov 27, 2025

View reviewed changes

src/tasks/WasmAppBuilder/WasmAppBuilder.csproj Show resolved Hide resolved

radekdoulik and others added 4 commits November 27, 2025 08:45

Feedback

f4d47bf

Co-authored-by: Adeel Mujahid <[email protected]>

Feedback

c1a754f

Merge remote-tracking branch 'remotes/origin/main' into clr-interp-wa…

fb8d531

…sm-callhelpers-generator-2nd-part

Update after merge

2fdf1e7

pavelsavara mentioned this pull request Dec 1, 2025

[browser][coreCLR] fix trimming of TimerHandler, BackgroundJobHandler #122068

Open

kg reviewed Dec 1, 2025

View reviewed changes

AaronRobinsonMSFT reviewed Dec 1, 2025

View reviewed changes

kg reviewed Dec 1, 2025

View reviewed changes

kg approved these changes Dec 1, 2025

View reviewed changes

[wasm][coreclr] 2nd part of call helpers generator, reverse calls #122007

Are you sure you want to change the base?

[wasm][coreclr] 2nd part of call helpers generator, reverse calls #122007

Conversation

radekdoulik commented Nov 26, 2025 • edited by AaronRobinsonMSFT Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

radekdoulik commented Nov 26, 2025 •

edited by AaronRobinsonMSFT

Loading